Inject shard-specific secrets into app compose templates (#138)#173
Open
ClaydeCode wants to merge 8 commits into
Open
Inject shard-specific secrets into app compose templates (#138)#173ClaydeCode wants to merge 8 commits into
ClaydeCode wants to merge 8 commits into
Conversation
App-scoped secrets are stored in a new app_secrets table keyed by (app_name, name). No foreign key to installed_apps: the DB snapshot restore inserts every table in one transaction in alphabetical order, so a non-deferrable FK (app_secrets sorts before installed_apps) would fail restore. Secrets are kept on uninstall by design, so there is no cascade to want anyway. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SecretResolver returns existing app-scoped secrets and mints new ones (32 chars from [A-Za-z0-9], via the secrets CSPRNG) synchronously so a compose template renders in a single jinja pass; new secrets are persisted afterwards. Secrets are reused, never rotated, so they stay stable across upgrades, uninstall/reinstall and restarts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Templates reference a secret with {{ secret('my-name') }}. The callable
form is chosen over {{ secret.my-name }} because jinja parses a dotted
name as attribute access (hyphens/dots break it) and a callable takes an
arbitrary key that cannot be mistaken for a fixed context key like fs or
portal. New secrets are persisted before the compose file is written, so a
persistence failure aborts the render rather than leaving a file with
unstored secrets. Secrets are kept on uninstall (nothing deletes them),
matching an app's retained user_data on reinstall.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers generation+persistence+injection, reuse across renders, distinct names getting distinct values, app-scoping (each app's secret isolated), retention on uninstall, secret-free templates persisting nothing, persist-before-write abort ordering, insert idempotency, and app_secrets riding along in the db_snapshot backup/restore round-trip. Each assertion was verified to fail when its code path is broken. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Also notes the honest limitation: app-scoping is not a boundary against a hostile app author because the compose template is rendered with a non-sandboxed jinja environment (as portal/fs already are). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The SandboxedEnvironment follow-up is not filed as an issue, so don't imply it is tracked; also add app_secrets to the DB tables list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The service module was named secrets.py, shadowing the stdlib secrets it uses (forcing a pysecrets alias) and mismatching the database.app_secrets module for the same concept. Rename to app_secrets.py; drop the alias. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…enerator Add a reinstall-reuse test (the headline lifecycle promise, previously only covered transitively), assert both secrets minted in one render pass are persisted, and unit-test generate_secret's length/charset/randomness directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #138.
What
Apps can now declare and receive shard-specific secrets in their
docker-compose.yml.template:On render,
secret('<name>')returns an app-scoped secret — generating one (32 chars,[A-Za-z0-9], via the stdlibsecretsCSPRNG) on first reference and persisting it in a newapp_secretstable. Names are arbitrary and declared purely by referencing them. A secret is stable across re-renders, upgrades, and restarts (reused, never rotated), kept on uninstall (so a reinstall reuses it, matching the app's retaineduser_data), and not encrypted at rest — matching the decisions in the issue thread.Templating syntax decision: the issue floated
{{ secret.my-secret }}; a Jinja attribute access breaks on hyphens (parses as subtraction), so this uses the callable{{ secret('name') }}form — the alternative the issue also blessed ("identifiers cannot be mistaken for fixed keys").The secret rides into the existing DB-snapshot backup automatically (
db_snapshot.pydiscovers all non-yoyo tables), so it survives a restore.How it works
render_docker_compose_template(app_installation/util.py) loads the app's existing secrets, renders with aSecretResolverthat mints missing secrets synchronously during the single Jinja pass (Jinja is sync, the DB is not), persists new secrets before writing the compose file (a persistence failure aborts the render rather than leaving a compose file with secrets that were never stored), then writes the file.Recommended reading order
migrations/shard-core-0002-app-secrets.sql— theapp_secretstable (composite PK(app_name, name), no FK toinstalled_appsso secrets survive uninstall)shard_core/database/app_secrets.py—get_all_for_app/ idempotentinsertshard_core/service/app_installation/app_secrets.py—generate_secret,SecretResolver, load/persistshard_core/service/app_installation/util.py— wiring into the render pathtests/test_app_secrets.py,tests/test_db_snapshot.py— coverageagents.md— documents the template context + honest limitationsVerification
tests/test_app_secrets.py(10 tests) andtests/test_db_snapshot.py(3) green.tests/test_app_installation.py(12, real Docker) green — confirms the change is safe on the real install path.ruff checkclean; full suite collects (236 tests, no import breakage).Review panel
Five reviewers ran (adversarial + test-adversary + DevEx + security + DB/migration), each given only the diff and the issue, instructed to refute. No blocking findings from any reviewer. Cheap advisories addressed:
secrets.pyshadowed stdlibsecrets(forced apysecretsalias) and mismatched the DB moduleapp_secrets.pyapp_secrets.py, dropped the alias (commit5480bed)test_distinct_namesasserted rendered values but never that both secrets minted in one pass were persisted (a persist-loop bug storing only the first would slip through)ced8b0d)test_secret_reused_after_reinstall(commitced8b0d)generate_secret()never unit-tested directly (charset check was probabilistic)ced8b0d)SandboxedEnvironmentfix — no such issue is filed277fe75)Advisories deliberately not taken, with reasons:
portal/fsalready render through the same non-sandboxedjinja2.Template, and custom-app upload is app-reachable per the architecture contract. This diff does not widen it:SecretResolveris built with only the current app's rows, sosecret('x')can never name another app's secret without the pre-existing escape. Honestly disclosed in agents.md; a realSandboxedEnvironmentfix is a separate, broader change.user_data, and rows are tiny.InstallationWorker, and boot re-render runs sequentially before the worker starts.test_app_installation.pyalready exercises the render path; skipped as costly for low marginal coverage.🤖 Generated with Claude Code